Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code Cleanup, Submodule Replacement, Error Handling Improvements & Deployment Enhancements #555

Merged
merged 70 commits into from
Sep 24, 2024

Conversation

dmccoystephenson
Copy link
Collaborator

PR Details

Description

This PR includes the following changes:

Code Cleanup:

  • Removed outdated "deposit over WebSocket to SDX" functionality.
  • Eliminated the ppm_tim service from Docker Compose files.

Submodule Updates:

  • Replaced the jpo-s3-deposit submodule with jpo-utils.

Error Handling Enhancements:

  • Improved error interpretation for "SNMP Error Code 10" from RSUs.
  • Stack traces for improperly encoded data from ACM are now printed only when debug logging is enabled.

Documentation and Configuration Updates:

  • Expanded project reference update documentation in release process document.
  • Updated mapfile references in ppm*.properties files.

Build and Deployment Improvements:

  • Resolved UID conflict for container builds.
  • Enabled Maven JAR publishing to GitHub Maven Central via GitHub Actions.

New Features:

  • Introduced a Docker startup script for log offloading via SSH/SCP.
  • Added support for source ASN1 bytes payloads for IMP depositors. This required an update to the message schemas, which necessitated a major version increment to v3.

Related Issue

No related GitHub issues.

Motivation and Context

The motivation behind these changes is to streamline the JPO-ODE project by removing outdated functionality, improving error handling, and simplifying deployment processes. By cleaning up unused services and submodules, the project becomes more maintainable, while enhanced error reporting increases transparency in debugging. The updates to the build and deployment pipeline, including resolving UID conflicts and automating JAR publishing, ensure smoother integration with modern workflows. Additionally, new features such as log offloading and support for source ASN1 bytes payloads improve operational efficiency and flexibility for users.

How Has This Been Tested?

These changes have been tested using unit tests, local deployments and the CDOT GCP deployments.

Types of changes

  • Defect fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that cause existing functionality to change)

Checklist:

  • I have added any new packages to the sonar-scanner.properties file
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
    ODE Contributing Guide
  • I have added tests to cover my changes.
  • All new and existing tests passed.

mwodahl and others added 30 commits May 29, 2024 10:00
Improve SNMP Error Code 10 Interpretation
Print stack trace upon receiving bad encoded data from ACM only if debug logging is enabled
…or-ppm

Updated mapfile reference in ppm*.properties files
@dmccoystephenson dmccoystephenson marked this pull request as ready for review September 9, 2024 16:22
@@ -17,3 +13,6 @@
[submodule "qa/test-harness/ode-output-validator-library"]
path = qa/test-harness/ode-output-validator-library
url = https://github.com/usdot-jpo-ode/ode-output-validator-library.git
[submodule "jpo-utils"]
path = jpo-utils
url = https://github.com/usdot-jpo-ode/jpo-utils.git
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SaikrishnaBairamoni Do we need to add github actions to scan this new repo?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dan-du-car we did not setup GitHub actions for jpo-utils repo seems like this is new, I can create a story for this in our backlog. Thanks!

@@ -28,7 +26,6 @@ jpo-ode-consumer-example.sonar.sources = src
jpo-ode-core.sonar.sources = src
jpo-ode-plugins.sonar.sources = src
jpo-ode-svcs.sonar.sources = src
jpo-s3-deposit.sonar.sources = src
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are replacing the s3 depositor with jpo-utils, is it possible to add jpo-util to this scanner? @SaikrishnaBairamoni

Copy link
Contributor

@SaikrishnaBairamoni SaikrishnaBairamoni Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can update the sonar properties as this PR changes are from CDOT fork I cannot make changes to this

import us.dot.its.jpo.ode.wrapper.HttpClientFactory.HttpException;
import us.dot.its.jpo.ode.wrapper.HttpClientFactory.HttpResponse;

public class CASClient {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we remove all the jpo-ode-core DDS components?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ODE previously used the DDS/WebSocket functionality to make deposits to the SDX. However, since the SDX no longer accepts deposits through this method, the DDS components are no longer needed.

@@ -0,0 +1,31 @@
name: Publish Java Package
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will be the approx. artifact size and where we are using this ? As our ODE GitHub org is on free plan we have artifact storage limit of 500mb.

Copy link
Contributor

@iyourshaw iyourshaw Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approx:
jpo-ode-common.jar - 38 KB
jpo-ode-core.jar - 154 KB
jpo-ode-plugins.jar - 436 KB
jpo-ode-svcs.jar - 134 MB (large because it includes the Spring web stuff. We could probably make it much smaller by only releasing the normal jar instead of the Spring Boot uberjar)

I'm not certain, but this seems to be saying that the storage limits for maven packages don't apply to public repos:
https://docs.github.com/en/billing/managing-billing-for-github-packages/about-billing-for-github-packages

I am fairly certain that there is no time limit on Maven packages (which are not the same as Gitub Action build artifacts)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked, and the normal jar for jpo-ode-svcs is approx 219KB, so if we changed the action to publish that, the total space would be less than 1MB for everything

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iyourshaw Got it, I'm good with this change. This storage limits are applied for only private repositories. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SaikrishnaBairamoni Here is the relevant PR that addresses this issue. We will work on getting this update merged ASAP.

Remove Sping Dependencies from Maven Jars
@dan-du-car dan-du-car merged commit bb847a3 into usdot-jpo-ode:develop Sep 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants